Ab fix out of bounds on set text #402
Conversation
* defensive change for null check * Fix for indexOutOfBounds on setText() from custom historyState.
|
So after some investigations, the problem is this code in handleRemovingCharacters function: val minRemoveIndex =
(textFieldValue.selection.max - removedCharsCount)
.coerceAtLeast(0)The remove doesn't work correctly if the previous selection.max is not the end of the removed text range. Changing it to this should fix the issue: val minRemoveIndex =
tempTextFieldValue.selection.min
.coerceAtLeast(0)I will create a PR with the fix and with test coverage for this scenario. |
| @@ -1741,20 +1758,29 @@ public class RichTextState internal constructor( | |||
| (!config.preserveStyleOnEmptyLine || richSpan.paragraph.isEmpty()) && | |||
| isSelectionAtNewRichSpan | |||
| ) { | |||
| newParagraphFirstRichSpan.spanStyle = SpanStyle() | |||
| newParagraphFirstRichSpan.richSpanStyle = RichSpanStyle.Default | |||
| if (newParagraphFirstRichSpan != null) { | |||
There was a problem hiding this comment.
There's no need for this check, if isSelectionAtNewRichSpan is true, it means that newParagraphFirstRichSpan is not null.
| } | ||
|
|
||
| removeTextRange(textRange) |
There was a problem hiding this comment.
After fixing the issue with removing characters, it should work fine. I prefer to keep using removeTextRange + addTextAfterSelection to avoid code duplication. It's not causing any issues since removeTextRange will change the selection to textRange.min.
| } | ||
|
|
||
| // A helper function to safely slice strings without throwing exceptions | ||
| private fun String.safeSubstring(startIndex: Int, endIndex: Int = this.length): String { |
There was a problem hiding this comment.
This helper function is useful, you can keep it even if it's going to be unused.
| @@ -304,20 +304,37 @@ public class RichTextState internal constructor( | |||
| textRange: TextRange, | |||
| text: String | |||
| ) { | |||
| require(textRange.min >= 0) { | |||
There was a problem hiding this comment.
I really prefer to force the user to provide a correct textRange.
With this new check we will get some unexpected results, let's say you enter (-10, -5) range then you well get an error saying "The start index (0) must be less than or equal to the end index (-5)." but the start index was -10 not 0.
Let's keep it the same.
This fixes a couple issues, replacing text range is independent from selection (cursor position) allowing for better programmatic replacement.
Add some defensive checks for nullable vars.
Finally, adding some safety around sub strings and slicing.